Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: ember@4 support #1256

Conversation

matthewhartstonge
Copy link
Contributor

@mansona it builds...
Haven't got to the point of running codemods/testing it yet as I just needed our apps to compile on ember@5 before circling back to upgrading/fixing ember-paper

@mansona
Copy link
Collaborator

mansona commented Oct 1, 2024

ooo nice one! 🎉 it looks like we're both thinking the same thing at the moment 😂

@matthewhartstonge
Copy link
Contributor Author

oop, looks like I need to merge in upstream..

@matthewhartstonge
Copy link
Contributor Author

matthewhartstonge commented Oct 1, 2024

Do you have contrib access to ember-css-transistions?
There is a bug in the repo that needs to be fixed:

  • utils aren't exported in the dist
  • the github action needs to be upgraded to strip the prepare script from the output package.json otherwise it gets run on npm install, causing a failure.

https://github.com/miguelcobain/ember-css-transitions/compare/master...matthewhartstonge:ember-css-transitions:master?expand=1

@mansona
Copy link
Collaborator

mansona commented Oct 2, 2024

I don't have access to ember-css-transitions no, but it looks like it's active enough so I wouldn't worry about opening up a PR there.

Why are you asking about utils though? It looks like the deployed version of the package is inlining the utils imports in the dist: https://unpkg.com/browse/ember-css-transitions@4.4.0/dist/modifiers/css-transition.js#L44

@matthewhartstonge
Copy link
Contributor Author

matthewhartstonge commented Oct 2, 2024

Cool, so the tests look like they are failing due to scope (probably non-explicit this e.t.c).

I'll look at patching that tomorrow. :)

not ok 54 Chrome 129.0 - [42 ms] - Integration | Component | paper-dialog: click outside >should close dialog if clickOutsideToClose
---
stack: >
Error: Attempted to resolve a value in a strict mode template, but that value was not in scope: dialogOpen

...

ok 308 Chrome 129.0 - [0 ms] - Unit | Validator | minlength: falsy values
ok 309 Chrome 129.0 - [0 ms] - Unit | Validator | required: strings
ok 310 Chrome 129.0 - [0 ms] - Unit | Validator | required: falsy values
ok 311 Chrome 129.0 - [0 ms] - ember-qunit: Ember.onerror validation: Ember.onerror is functioning properly

1..311
tests 311
pass 30
skip 4
todo 0
fail 277

@mansona
Copy link
Collaborator

mansona commented Oct 3, 2024

So if we get a small pr of yours merged then you won't need me to approve the tests any more.

Maybe extract the ember-css-transition upgrade on its own

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants